Get rid of polling in WarpSync
#1265
Conversation
dmitry-markin
added
R0-silent
| /// A channel to get target block header if we skip over proofs downloading during warp sync. | ||
| warp_sync_target_block_header_rx: Option<oneshot::Receiver<<B as BlockT>::Header>>, | ||
|
|
There was a problem hiding this comment.
I think this is fine for now but in the future there should be some kind of syncing strategy selection like:
enum Strategy {
Warp(WarpSync),
Full(FullSync),
Whatever(...)
}
enum Action {
SendRequest(Vec<u8>),
SendResponse(Vec<u8>),
ImportBlock(Block),
}
enum SyncEvent {
ResponseReceived(Vec<u8>),
}
trait Strategy {
fn register_event(&mut self, event: SyncEvent);
fn next_action(&mut self) -> Option<SyncAction>;
}
impl SyncingEngine {
async fn run(mut self) {
loop {
tokio::select! {
response = self.pending_responses.select_next_some() => {
self.strategy.register_event(SyncEvent::ResponseReceived(response));
}
}
if let Some(action) = self.strategy.next_action() {
// TODO: perform action
}
}
}
}Details of the syncing algorithm would be opaque to SyncingEngine and it would only deal with I/O and dispatch the events to the correct syncing strategy which would deal with the logic of that particular strategy and emit I/O-related events to SyncingEngine. How feasible this is to remains to be seen but worth looking into.
There was a problem hiding this comment.
Yeah that is a good idea. Goes into the direction of: #532
There was a problem hiding this comment.
@altonen Thanks for the snipped provided! My idea was to gradually move all polling to the upper levels (SyncingEngine) first, otherwise it's difficult to do everything in one go. And WarpSync requiring polling was quite an annoying part, so I started from it.
There was a problem hiding this comment.
Yes I agree this shouldn't be done in this PR and I think your approach of first gradually removing stuff away from ChainSync is good but I just wanted to highlight what the ultimate plan is.
I've only checked that warp sync tests are passing (both regular and "target block"). I'll manually check polkadot warp sync too. To be honest, I don't know how to test parachain warp sync. Do we have some "default" parachain full node in cumulus for such tests? |
|
@altonen both relay- and parachain warp sync works (tested with |
|
Same here, any reason this isn't merged? |
Also due to a failing zombienet test. |
|
bot merge |
|
@dmitry-markin Unknown command "merge". Refer to help docs and/or source code. |
* master: (24 commits) GHW for building and publishing docker images (#1391) pallet asset-conversion additional quote tests (#1371) Remove deprecated `pallet_balances`'s `set_balance_deprecated` and `transfer` dispatchables (#1226) Fix PRdoc check (#1419) Fix the wasm runtime substitute caching bug (#1416) Bump enumn from 0.1.11 to 0.1.12 (#1412) RFC 14: Improve locking mechanism for parachains (#1290) Add PRdoc check (#1408) fmt fixes (#1413) Enforce a decoding limit in MultiAssets (#1395) Remove dynamic dispatch using `Ext` (#1399) Remove redundant calls to `borrow()` (#1393) Get rid of polling in `WarpSync` (#1265) Bump actions/checkout from 3 to 4 (#1398) Bump thiserror from 1.0.47 to 1.0.48 (#1396) Move Relay-Specific Shared Code to One Place (#1193) rust docs: add simple analytics (#1377) Contracts: Update read_sandbox (#1390) Extract block announce validation from `ChainSync` (#1170) [ci] Remove runtime-benchmarks from tests (#1335) ...
* master: (28 commits) Adds base benchmark for do_tick in broker pallet (#1235) zombienet: use another collator image for the slashing test (#1386) Prevent a fail prdoc check to block (#1433) Fix nothing scheduled on session boundary (#1403) GHW for building and publishing docker images (#1391) pallet asset-conversion additional quote tests (#1371) Remove deprecated `pallet_balances`'s `set_balance_deprecated` and `transfer` dispatchables (#1226) Fix PRdoc check (#1419) Fix the wasm runtime substitute caching bug (#1416) Bump enumn from 0.1.11 to 0.1.12 (#1412) RFC 14: Improve locking mechanism for parachains (#1290) Add PRdoc check (#1408) fmt fixes (#1413) Enforce a decoding limit in MultiAssets (#1395) Remove dynamic dispatch using `Ext` (#1399) Remove redundant calls to `borrow()` (#1393) Get rid of polling in `WarpSync` (#1265) Bump actions/checkout from 3 to 4 (#1398) Bump thiserror from 1.0.47 to 1.0.48 (#1396) Move Relay-Specific Shared Code to One Place (#1193) ...
This PR is part of Sync 2.0 preparation work and a step towards
ChainSyncas a pure state machine.Refs #534